Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: change the AI command line argument to optional #617

Merged
merged 10 commits into from
Sep 24, 2024

Conversation

jueli12
Copy link

@jueli12 jueli12 commented Sep 18, 2024

What type of PR is this?

/kind feature

What this PR does / why we need it:

change the AI command line argument to optional

Which issue(s) this PR fixes:

Fixes #551

@jueli12 jueli12 changed the base branch from main to support-natural-language September 18, 2024 14:02
@@ -69,7 +67,12 @@ func SearchForResource(searchMgr *search.SearchManager, aiMgr *ai.AIManager, sea
query := searchQuery

if searchPattern == storage.NLPatternType {
//logger.Info(searchQuery)
// logger.Info(searchQuery)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Useless comments can be removed

@@ -25,7 +25,10 @@ type AIManager struct {

// NewAIManager returns a new AIManager object
func NewAIManager(c registry.ExtraConfig) (*AIManager, error) {
aiClient := ai.NewClient(c.Backend)
if c.AIAuthToken == "" {
return nil, nil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the token is empty, an error should be returned, otherwise the npe error is prone to occur

Copy link
Author

@jueli12 jueli12 Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When Karpor starts, it creates a new AIManager. If return the error, the startup will fail. Now I am re-evaluating whether it is null when calling the aimanager function. Is there any other way to do this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be up to the caller to decide which errors to ignore. The underlying method should ensure that the returned entity is not empty when there are no errors.

Copy link
Collaborator

@elliotxx elliotxx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@elliotxx elliotxx merged commit dbf281e into KusionStack:support-natural-language Sep 24, 2024
3 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants